Skip to content

Fix retrying logic for HTTP responses with "Content-Type" other than "application/json" - #2329

Merged
vam-google merged 1 commit into
googleapis:masterfrom
vam-google:master
Aug 11, 2017
Merged

Fix retrying logic for HTTP responses with "Content-Type" other than "application/json"#2329
vam-google merged 1 commit into
googleapis:masterfrom
vam-google:master

Conversation

@vam-google

Copy link
Copy Markdown
Contributor

This should fix #2098

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 10, 2017
@garrettjonesgoogle

Copy link
Copy Markdown
Contributor

How does it get to the exception instanceof GoogleJsonResponseException branch if Content-Type is not application/json?

@vam-google

Copy link
Copy Markdown
Contributor Author

This is caused by (I believe) some key design decisions in google-api-client. When you make a http request you may define it as a JSON request (the request class extends AbstractGoogleJsonClientRequest, and it is the case for StorageRequest). Unfortunately, the code is structured in a way that if your request is a JsonRequest, then it assumes that your error responses are also in Json.

Places, where things start going in a wrong direction are:

But regardless of these details, I believe the proposed fix is not only the simplest but at the same time the best we can have practically. In our clients retries are configured by error codes, not by content type. The current code, for some reason starts ignoring the error code if it cannot parse the message. Why? Error code should sill be treated as error code. It is ok to try to get more information from the message (it potentially allows to handle error better), bit if that does not succeed, we should fallback to checking just error code.

@garrettjonesgoogle

Copy link
Copy Markdown
Contributor

ok, LGTM

@vam-google
vam-google merged commit 22e5423 into googleapis:master Aug 11, 2017
chingor13 pushed a commit that referenced this pull request Feb 20, 2026
suztomo pushed a commit to suztomo/google-cloud-java that referenced this pull request Mar 23, 2026
meltsufin pushed a commit that referenced this pull request Apr 29, 2026
meltsufin pushed a commit that referenced this pull request May 1, 2026
meltsufin pushed a commit that referenced this pull request May 2, 2026
benjaminp pushed a commit to benjaminp/google-cloud-java that referenced this pull request Jul 13, 2026
* Introduce ConsistencyParams model

Change-Id: I7ae07cc4f13e8ffe9ea4a55fb407eae1d64f8547

* Create AwaitConsistencyCallable, a delegate for AwaitReplicationCallable

Change-Id: I16d7c69b4e1b9153f93e83f7f846d6c172aae8a6

* Address some PR comments

Change-Id: Icdf94f2f6a13f55d2c9204774d4ebbac5aa6a8b3

* Remove unused imports from AwaitReplicationCallable

Change-Id: Ia4861f1a5796061ca86844c67c68f54711fdbb94

* Plumb the Consistency callable through to some places, add some tests

Change-Id: Ibe60e2a1044933af1008c0cd1b84f757dd6867a8

* Add integration test

Change-Id: Ie3b2b2983ca585cb1d6a2cdb8b18b55e81205759

* Rework the ConsistencyRequest model, plumb through RequestContext to BigtableTableAdminClient

Change-Id: I840282587d3d6cb4150dfbdd568c347dc32a732d

* Fix imports

Change-Id: Ic7588b3d04877a56089c23036d6df73a5c9b0cd5

* Fix more imports, fix some tests

Change-Id: I2723fd67bd301a4eb3aeae80d91fa663cdd6ab01

* Rename some things

Change-Id: Ie1bc8478c418d49b0c2e014edbeb6f56b56b0dd1

* Add tests for ConsistencyRequest model

Change-Id: I3548b7aa673be5a92cd4c180e3edb8649657811c

* Add newline

Change-Id: Icdd22ce2857e5b4316c6fa3f0e139ea9de825178

* Fix broken test

Change-Id: Idbd7c0f10ebe575d104ab7ac46a3a1e347e35fe8

* Make request context a final variable in test

Change-Id: I81f2a25fe4493021bab150ab0af65d7318ba2399

* Get test working using correct expectations

Change-Id: Ie34d5171bd7a472fc695d603849e260054aedfbd

* Add a couple of tests for AwaitReplicationCallable

Change-Id: I70014db2c0a1d4e74c23b18de7ef591bc70cda2a

* Use RequestContextNoAP class

Change-Id: I897b343cd1067d43bcc644cac3db44e88bbf1e69

* Make ConsistencyRequest model an AutoValue

Change-Id: I9529fb79da69e12a834a2d0fea032d72ae6ea157

* Fix license year, fix some formatting

Change-Id: Ibcca1ca9f49988764fdbeeacc59cac5d276ab266

* Run auto formatter

Change-Id: I9f5e3f7c7fd79262092c507a523e16a533bc4382

* Rename new RequestContext to TableAdminRequestContext, re run auto formatter

Change-Id: Ib3f5918ef0f5b1ac53147baf93dcb72c476d877b

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* Add license header to ConsistencyRequestTest

Change-Id: I733d2f8c082647ad32b72b04b218cd5ba79d2377

* Add EnhancedBigtableTableAdminStub to clirr-ignored-differences

Change-Id: I7eefeda777305dd3d7c5664097bda87ac63daa72

* Fix IT tests, skip data boost one for now until we run it concurrently

Change-Id: I764190b0f91614753080e0a96e7e11e3dfb1fde0

* Run autoformatter

Change-Id: Iba4671e4781f1b333279a2410563869f53b284d5

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storage retries don't work as expected

3 participants